Skip to content

Conversation

@fmeum
Copy link
Member

@fmeum fmeum commented Feb 16, 2025

What type of PR is this?

Feature

What does this PR do? Why is it needed?

Which issues(s) does this PR fix?

Work towards #4241

Other notes for review

@fmeum fmeum force-pushed the 4241-all-nogo-scope branch from 6d30e40 to f5a8014 Compare February 17, 2025 08:50
@fmeum
Copy link
Member Author

fmeum commented Feb 17, 2025

@fionera Could you give this a try as well? I'm not sure how usable it is though, the first nogo finding is:

nogo: errors found by nogo during build-time code analysis:
external/gazelle~~go_deps~org_golang_x_sys/unix/syscall_darwin.go:584:19: declaration of "err" shadows declaration at line 578 (shadow)
external/gazelle~~go_deps~org_golang_x_sys/unix/syscall_darwin.go:593:19: declaration of "err" shadows declaration at line 578 (shadow)
external/gazelle~~go_deps~org_golang_x_sys/unix/syscall_unix.go:117:28: possible misuse of unsafe.Pointer (unsafeptr)
external/gazelle~~go_deps~org_golang_x_sys/unix/syscall_unix.go:159:9: possible misuse of unsafe.Pointer (unsafeptr)
external/gazelle~~go_deps~org_golang_x_sys/unix/sysvshm_unix.go:32:28: possible misuse of unsafe.Pointer (unsafeptr)

How are you dealing with this amount of warnings in basic deps?

@fionera
Copy link
Contributor

fionera commented Feb 18, 2025

@fmeum Works fine, except that our excludes are not working anymore and I only just noticed. 😄 Because of the new naming of bzlmod didn't appear as issue before this patch. Since you asked how we are dealing with them, we have a fairly big exception list https://github.com/monogon-dev/monogon/blob/main/build/analysis/BUILD.bazel Is there motivation to replace the bzlmod repo name with the actual go dependency name for these? Or is that not really possible because of the way the excludes work?

@fmeum
Copy link
Member Author

fmeum commented Feb 18, 2025

Have you tried the excludes attribute of the nogo tag? It accepts labels, not paths, and should this work with Bzlmod.

@fionera
Copy link
Contributor

fionera commented Feb 21, 2025

That would exclude them completely. I only want to exclude them from specific nogo analyzers.

@fmeum
Copy link
Member Author

fmeum commented Feb 21, 2025

Could you file a separate issue for that? We probably do need to change how we identify files in the nogo config

@fionera
Copy link
Contributor

fionera commented Feb 25, 2025

Opened #4284 for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants